Verify cross-platform claim in CI + fix filepath.Match on Windows#7
Merged
Verify cross-platform claim in CI + fix filepath.Match on Windows#7
Conversation
Two related changes that close the gap between "cross-compiles" and
"actually works on Windows/macOS":
1. CI now runs go vet + go test on ubuntu, macos, and windows runners
via a matrix strategy. Release workflow has been publishing five
OS/arch binaries, but CI only ever ran on ubuntu — any regression
from filepath semantics, signal handling, or temp-file quirks
would have surfaced in a user report, not a PR. Split into `test`
(portable) and `dogfood` (linux-only, uses make + ./gitcortex) so
the quality-gate steps don't need POSIX-tooling workarounds.
2. ShouldIgnore used filepath.Match, which is OS-aware and uses the
platform separator ("\\" on Windows) for glob metacharacters.
Paths from git arrive forward-slash on every OS, so a pattern
like "src/generated/*.go" could silently fail to match
"src/generated/types.go" on Windows. Swapped to path.Match (the
URL/unix-style variant that always uses "/"). The parameter `path`
was renamed to `p` to avoid shadowing the stdlib package name.
Test fixture gained two cases covering the multi-segment glob
behavior that motivated the fix; all existing cases still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related changes that close the gap between "cross-compiles" and "actually works on Windows/macOS":
CI now runs go vet + go test on ubuntu, macos, and windows runners via a matrix strategy. Release workflow has been publishing five OS/arch binaries, but CI only ever ran on ubuntu — any regression from filepath semantics, signal handling, or temp-file quirks would have surfaced in a user report, not a PR. Split into
test(portable) anddogfood(linux-only, uses make + ./gitcortex) so the quality-gate steps don't need POSIX-tooling workarounds.ShouldIgnore used filepath.Match, which is OS-aware and uses the platform separator ("\" on Windows) for glob metacharacters. Paths from git arrive forward-slash on every OS, so a pattern like "src/generated/*.go" could silently fail to match "src/generated/types.go" on Windows. Swapped to path.Match (the URL/unix-style variant that always uses "/"). The parameter
pathwas renamed topto avoid shadowing the stdlib package name.Test fixture gained two cases covering the multi-segment glob behavior that motivated the fix; all existing cases still pass.